[libc] Modular printf option (float only) #147426
Draft
+216
−33
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-libc Author: Daniel Thornburgh (mysterymath) ChangesThis adds LIBC_CONF_PRINTF_MODULAR, which causes floating point support (later, others) to be weakly linked into the implementation. __printf_modular becomes the main entry point of the implementaiton, an printf itself wraps __printf_modular. printf it also contains a BFD_RELOC_NONE relocation to bring in the float aspect. See issue #146159 for context. Full diff: https://github.com/llvm/llvm-project/pull/147426.diff 14 Files Affected:
diff --git a/libc/config/config.json b/libc/config/config.json
index d53b2936edb07..4278169cd5940 100644
--- a/libc/config/config.json
+++ b/libc/config/config.json
@@ -45,6 +45,10 @@
"LIBC_CONF_PRINTF_RUNTIME_DISPATCH": {
"value": true,
"doc": "Use dynamic dispatch for the output mechanism to reduce code size."
+ },
+ "LIBC_CONF_PRINTF_MODULAR": {
+ "value": true,
+ "doc": "Split printf implementation into modules that can be lazily linked in."
}
},
"scanf": {
diff --git a/libc/docs/configure.rst b/libc/docs/configure.rst
index 109412225634f..1998c067dc77a 100644
--- a/libc/docs/configure.rst
+++ b/libc/docs/configure.rst
@@ -45,6 +45,7 @@ to learn about the defaults for your platform and target.
- ``LIBC_CONF_PRINTF_FLOAT_TO_STR_USE_DYADIC_FLOAT``: Use dyadic float for faster and smaller but less accurate printf doubles.
- ``LIBC_CONF_PRINTF_FLOAT_TO_STR_USE_FLOAT320``: Use an alternative printf float implementation based on 320-bit floats
- ``LIBC_CONF_PRINTF_FLOAT_TO_STR_USE_MEGA_LONG_DOUBLE_TABLE``: Use large table for better printf long double performance.
+ - ``LIBC_CONF_PRINTF_MODULAR``: Split printf implementation into modules that can be lazily linked in.
- ``LIBC_CONF_PRINTF_RUNTIME_DISPATCH``: Use dynamic dispatch for the output mechanism to reduce code size.
* **"pthread" options**
- ``LIBC_CONF_RAW_MUTEX_DEFAULT_SPIN_COUNT``: Default number of spins before blocking if a mutex is in contention (default to 100).
diff --git a/libc/src/stdio/generic/CMakeLists.txt b/libc/src/stdio/generic/CMakeLists.txt
index 6361822b61999..41b18bc7195ca 100644
--- a/libc/src/stdio/generic/CMakeLists.txt
+++ b/libc/src/stdio/generic/CMakeLists.txt
@@ -412,10 +412,15 @@ if(LLVM_LIBC_FULL_BUILD)
)
endif()
+set(printf_srcs printf.cpp)
+if (LIBC_CONF_PRINTF_MODULAR)
+ list(APPEND printf_srcs printf_modular.cpp)
+endif()
+
add_generic_entrypoint_object(
printf
SRCS
- printf.cpp
+ ${printf_srcs}
HDRS
../printf.h
DEPENDS
diff --git a/libc/src/stdio/generic/printf_modular.cpp b/libc/src/stdio/generic/printf_modular.cpp
new file mode 100644
index 0000000000000..3a6a580002062
--- /dev/null
+++ b/libc/src/stdio/generic/printf_modular.cpp
@@ -0,0 +1,40 @@
+//===-- Implementation of printf_modular-----------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "src/stdio/printf.h"
+
+#include "src/__support/File/file.h"
+#include "src/__support/arg_list.h"
+#include "src/__support/macros/config.h"
+#include "src/stdio/printf_core/vfprintf_internal.h"
+
+#include "hdr/types/FILE.h"
+#include <stdarg.h>
+
+#ifndef LIBC_COPT_STDIO_USE_SYSTEM_FILE
+#define PRINTF_STDOUT LIBC_NAMESPACE::stdout
+#else // LIBC_COPT_STDIO_USE_SYSTEM_FILE
+#define PRINTF_STDOUT ::stdout
+#endif // LIBC_COPT_STDIO_USE_SYSTEM_FILE
+
+namespace LIBC_NAMESPACE_DECL {
+
+LLVM_LIBC_FUNCTION(int, __printf_modular,
+ (const char *__restrict format, ...)) {
+ va_list vlist;
+ va_start(vlist, format);
+ internal::ArgList args(vlist); // This holder class allows for easier copying
+ // and pointer semantics, as well as handling
+ // destruction automatically.
+ va_end(vlist);
+ int ret_val = printf_core::vfprintf_internal_modular(
+ reinterpret_cast<::FILE *>(PRINTF_STDOUT), format, args);
+ return ret_val;
+}
+
+} // namespace LIBC_NAMESPACE_DECL
diff --git a/libc/src/stdio/printf.h b/libc/src/stdio/printf.h
index 9e47ad8680f9c..81b7d866a6a59 100644
--- a/libc/src/stdio/printf.h
+++ b/libc/src/stdio/printf.h
@@ -15,6 +15,7 @@
namespace LIBC_NAMESPACE_DECL {
int printf(const char *__restrict format, ...);
+int __printf_modular(const char *__restrict format, ...);
} // namespace LIBC_NAMESPACE_DECL
diff --git a/libc/src/stdio/printf_core/CMakeLists.txt b/libc/src/stdio/printf_core/CMakeLists.txt
index c22f9858f3b1e..abc4cc8269d79 100644
--- a/libc/src/stdio/printf_core/CMakeLists.txt
+++ b/libc/src/stdio/printf_core/CMakeLists.txt
@@ -28,6 +28,9 @@ endif()
if(LIBC_CONF_PRINTF_RUNTIME_DISPATCH)
list(APPEND printf_config_copts "-DLIBC_COPT_PRINTF_RUNTIME_DISPATCH")
endif()
+if(LIBC_CONF_PRINTF_MODULAR)
+ list(APPEND printf_config_copts "-DLIBC_COPT_PRINTF_MODULAR")
+endif()
if(printf_config_copts)
list(PREPEND printf_config_copts "COMPILE_OPTIONS")
endif()
@@ -112,10 +115,12 @@ add_header_library(
libc.src.__support.StringUtil.error_to_string
)
-add_header_library(
+add_object_library(
printf_main
HDRS
printf_main.h
+ SRCS
+ float_impl.cpp
DEPENDS
.parser
.converter
diff --git a/libc/src/stdio/printf_core/float_dec_converter.h b/libc/src/stdio/printf_core/float_dec_converter.h
index ed004f9a26a13..deeb566bd4092 100644
--- a/libc/src/stdio/printf_core/float_dec_converter.h
+++ b/libc/src/stdio/printf_core/float_dec_converter.h
@@ -1122,11 +1122,23 @@ LIBC_INLINE int convert_float_dec_auto_typed(Writer<write_mode> *writer,
}
}
+template <WriteMode write_mode>
+LIBC_PRINTF_MODULAR_DECL int
+convert_float_decimal(Writer<write_mode> *writer, const FormatSection &to_conv);
+template <WriteMode write_mode>
+LIBC_PRINTF_MODULAR_DECL int
+convert_float_dec_exp(Writer<write_mode> *writer, const FormatSection &to_conv);
+template <WriteMode write_mode>
+LIBC_PRINTF_MODULAR_DECL int
+convert_float_dec_auto(Writer<write_mode> *writer,
+ const FormatSection &to_conv);
+
+#ifdef LIBC_PRINTF_DEFINE_MODULAR
// TODO: unify the float converters to remove the duplicated checks for inf/nan.
template <WriteMode write_mode>
-LIBC_INLINE int convert_float_decimal(Writer<write_mode> *writer,
- const FormatSection &to_conv) {
+int convert_float_decimal(Writer<write_mode> *writer,
+ const FormatSection &to_conv) {
if (to_conv.length_modifier == LengthModifier::L) {
fputil::FPBits<long double>::StorageType float_raw = to_conv.conv_val_raw;
fputil::FPBits<long double> float_bits(float_raw);
@@ -1147,8 +1159,8 @@ LIBC_INLINE int convert_float_decimal(Writer<write_mode> *writer,
}
template <WriteMode write_mode>
-LIBC_INLINE int convert_float_dec_exp(Writer<write_mode> *writer,
- const FormatSection &to_conv) {
+int convert_float_dec_exp(Writer<write_mode> *writer,
+ const FormatSection &to_conv) {
if (to_conv.length_modifier == LengthModifier::L) {
fputil::FPBits<long double>::StorageType float_raw = to_conv.conv_val_raw;
fputil::FPBits<long double> float_bits(float_raw);
@@ -1169,8 +1181,8 @@ LIBC_INLINE int convert_float_dec_exp(Writer<write_mode> *writer,
}
template <WriteMode write_mode>
-LIBC_INLINE int convert_float_dec_auto(Writer<write_mode> *writer,
- const FormatSection &to_conv) {
+int convert_float_dec_auto(Writer<write_mode> *writer,
+ const FormatSection &to_conv) {
if (to_conv.length_modifier == LengthModifier::L) {
fputil::FPBits<long double>::StorageType float_raw = to_conv.conv_val_raw;
fputil::FPBits<long double> float_bits(float_raw);
@@ -1189,6 +1201,7 @@ LIBC_INLINE int convert_float_dec_auto(Writer<write_mode> *writer,
return convert_inf_nan(writer, to_conv);
}
+#endif
} // namespace printf_core
} // namespace LIBC_NAMESPACE_DECL
diff --git a/libc/src/stdio/printf_core/float_dec_converter_limited.h b/libc/src/stdio/printf_core/float_dec_converter_limited.h
index f468dbc8e2ae8..9804a38964be0 100644
--- a/libc/src/stdio/printf_core/float_dec_converter_limited.h
+++ b/libc/src/stdio/printf_core/float_dec_converter_limited.h
@@ -676,22 +676,34 @@ LIBC_INLINE int convert_float_dec_auto_typed(Writer<write_mode> *writer,
}
template <WriteMode write_mode>
-LIBC_INLINE int convert_float_decimal(Writer<write_mode> *writer,
- const FormatSection &to_conv) {
+LIBC_PRINTF_MODULAR_DECL int convert_float_decimal(Writer<write_mode> *writer,
+ const FormatSection &to_conv);
+template <WriteMode write_mode>
+LIBC_PRINTF_MODULAR_DECL int convert_float_dec_exp(Writer<write_mode> *writer,
+ const FormatSection &to_conv);
+template <WriteMode write_mode>
+LIBC_PRINTF_MODULAR_DECL int convert_float_dec_auto(Writer<write_mode> *writer,
+ const FormatSection &to_conv);
+
+#ifdef LIBC_PRINTF_DEFINE_MODULAR
+template <WriteMode write_mode>
+int convert_float_decimal(Writer<write_mode> *writer,
+ const FormatSection &to_conv) {
return convert_float_outer(writer, to_conv, ConversionType::F);
}
template <WriteMode write_mode>
-LIBC_INLINE int convert_float_dec_exp(Writer<write_mode> *writer,
- const FormatSection &to_conv) {
+int convert_float_dec_exp(Writer<write_mode> *writer,
+ const FormatSection &to_conv) {
return convert_float_outer(writer, to_conv, ConversionType::E);
}
template <WriteMode write_mode>
-LIBC_INLINE int convert_float_dec_auto(Writer<write_mode> *writer,
- const FormatSection &to_conv) {
+int convert_float_dec_auto(Writer<write_mode> *writer,
+ const FormatSection &to_conv) {
return convert_float_outer(writer, to_conv, ConversionType::G);
}
+#endif
} // namespace printf_core
} // namespace LIBC_NAMESPACE_DECL
diff --git a/libc/src/stdio/printf_core/float_hex_converter.h b/libc/src/stdio/printf_core/float_hex_converter.h
index 16592e7bac932..fa724066813d7 100644
--- a/libc/src/stdio/printf_core/float_hex_converter.h
+++ b/libc/src/stdio/printf_core/float_hex_converter.h
@@ -26,8 +26,13 @@ namespace LIBC_NAMESPACE_DECL {
namespace printf_core {
template <WriteMode write_mode>
-LIBC_INLINE int convert_float_hex_exp(Writer<write_mode> *writer,
- const FormatSection &to_conv) {
+LIBC_PRINTF_MODULAR_DECL int convert_float_hex_exp(Writer<write_mode> *writer,
+ const FormatSection &to_conv);
+
+#ifdef LIBC_PRINTF_DEFINE_MODULAR
+template <WriteMode write_mode>
+int convert_float_hex_exp(Writer<write_mode> *writer,
+ const FormatSection &to_conv) {
using LDBits = fputil::FPBits<long double>;
using StorageType = LDBits::StorageType;
@@ -254,6 +259,7 @@ LIBC_INLINE int convert_float_hex_exp(Writer<write_mode> *writer,
}
return WRITE_OK;
}
+#endif
} // namespace printf_core
} // namespace LIBC_NAMESPACE_DECL
diff --git a/libc/src/stdio/printf_core/float_impl.cpp b/libc/src/stdio/printf_core/float_impl.cpp
new file mode 100644
index 0000000000000..e7c9ba39aa148
--- /dev/null
+++ b/libc/src/stdio/printf_core/float_impl.cpp
@@ -0,0 +1,41 @@
+#ifdef LIBC_COPT_PRINTF_MODULAR
+#include "src/__support/arg_list.h"
+
+#define LIBC_PRINTF_DEFINE_MODULAR
+#include "src/stdio/printf_core/float_dec_converter.h"
+#include "src/stdio/printf_core/float_hex_converter.h"
+#include "src/stdio/printf_core/parser.h"
+
+namespace LIBC_NAMESPACE_DECL {
+namespace printf_core {
+template class Parser<internal::ArgList>;
+template class Parser<internal::DummyArgList<false>>;
+template class Parser<internal::DummyArgList<true>>;
+template class Parser<internal::StructArgList<false>>;
+template class Parser<internal::StructArgList<true>>;
+
+#define INSTANTIATE_CONVERT_FN(NAME) \
+ template int NAME<WriteMode::FILL_BUFF_AND_DROP_OVERFLOW>( \
+ Writer<WriteMode::FILL_BUFF_AND_DROP_OVERFLOW> * writer, \
+ const FormatSection &to_conv); \
+ template int NAME<WriteMode::FLUSH_TO_STREAM>( \
+ Writer<WriteMode::FLUSH_TO_STREAM> * writer, \
+ const FormatSection &to_conv); \
+ template int NAME<WriteMode::RESIZE_AND_FILL_BUFF>( \
+ Writer<WriteMode::RESIZE_AND_FILL_BUFF> * writer, \
+ const FormatSection &to_conv); \
+ template int NAME<WriteMode::RUNTIME_DISPATCH>( \
+ Writer<WriteMode::RUNTIME_DISPATCH> * writer, \
+ const FormatSection &to_conv)
+
+INSTANTIATE_CONVERT_FN(convert_float_decimal);
+INSTANTIATE_CONVERT_FN(convert_float_dec_exp);
+INSTANTIATE_CONVERT_FN(convert_float_dec_auto);
+INSTANTIATE_CONVERT_FN(convert_float_hex_exp);
+
+} // namespace printf_core
+} // namespace LIBC_NAMESPACE_DECL
+
+// Bring this file into the link if __printf_float is referenced.
+extern "C" void __printf_float() {}
+#endif
diff --git a/libc/src/stdio/printf_core/parser.h b/libc/src/stdio/printf_core/parser.h
index cef9b1ae58fa0..5a1eea36b603f 100644
--- a/libc/src/stdio/printf_core/parser.h
+++ b/libc/src/stdio/printf_core/parser.h
@@ -236,11 +236,7 @@ template <typename ArgProvider> class Parser {
case ('A'):
case ('g'):
case ('G'):
- if (lm != LengthModifier::L) {
- WRITE_ARG_VAL_SIMPLEST(section.conv_val_raw, double, conv_index);
- } else {
- WRITE_ARG_VAL_SIMPLEST(section.conv_val_raw, long double, conv_index);
- }
+ write_float_arg_val(section, lm, conv_index);
break;
#endif // LIBC_COPT_PRINTF_DISABLE_FLOAT
#ifdef LIBC_INTERNAL_PRINTF_HAS_FIXED_POINT
@@ -299,6 +295,12 @@ template <typename ArgProvider> class Parser {
return section;
}
+ LIBC_PRINTF_MODULAR_DECL void write_float_arg_val(FormatSection §ion,
+ LengthModifier lm,
+ size_t conv_index);
+ LIBC_PRINTF_MODULAR_DECL TypeDesc float_type_desc(LengthModifier lm);
+ LIBC_PRINTF_MODULAR_DECL bool advance_arg_if_float(TypeDesc cur_type_desc);
+
private:
// parse_flags parses the flags inside a format string. It assumes that
// str[*local_pos] is inside a format specifier, and parses any flags it
@@ -474,10 +476,9 @@ template <typename ArgProvider> class Parser {
args_cur.template next_var<uint64_t>();
#ifndef LIBC_COPT_PRINTF_DISABLE_FLOAT
// Floating point numbers are stored separately from the other arguments.
- else if (cur_type_desc == type_desc_from_type<double>())
- args_cur.template next_var<double>();
- else if (cur_type_desc == type_desc_from_type<long double>())
- args_cur.template next_var<long double>();
+ else if (&Parser::advance_arg_if_float &&
+ advance_arg_if_float(cur_type_desc))
+ ;
#endif // LIBC_COPT_PRINTF_DISABLE_FLOAT
#ifdef LIBC_INTERNAL_PRINTF_HAS_FIXED_POINT
// Floating point numbers may be stored separately from the other
@@ -630,10 +631,7 @@ template <typename ArgProvider> class Parser {
case ('A'):
case ('g'):
case ('G'):
- if (lm != LengthModifier::L)
- conv_size = type_desc_from_type<double>();
- else
- conv_size = type_desc_from_type<long double>();
+ conv_size = float_type_desc(lm);
break;
#endif // LIBC_COPT_PRINTF_DISABLE_FLOAT
#ifdef LIBC_INTERNAL_PRINTF_HAS_FIXED_POINT
@@ -682,6 +680,38 @@ template <typename ArgProvider> class Parser {
#endif // LIBC_COPT_PRINTF_DISABLE_INDEX_MODE
};
+#ifdef LIBC_PRINTF_DEFINE_MODULAR
+template <typename ArgParser>
+void Parser<ArgParser>::write_float_arg_val(FormatSection §ion,
+ LengthModifier lm,
+ size_t conv_index) {
+ if (lm != LengthModifier::L) {
+ WRITE_ARG_VAL_SIMPLEST(section.conv_val_raw, double, conv_index);
+ } else {
+ WRITE_ARG_VAL_SIMPLEST(section.conv_val_raw, long double, conv_index);
+ }
+}
+
+template <typename ArgParser>
+TypeDesc Parser<ArgParser>::float_type_desc(LengthModifier lm) {
+ if (lm != LengthModifier::L)
+ return type_desc_from_type<double>();
+ else
+ return type_desc_from_type<long double>();
+}
+
+template <typename ArgParser>
+bool Parser<ArgParser>::advance_arg_if_float(TypeDesc cur_type_desc) {
+ if (cur_type_desc == type_desc_from_type<double>())
+ args_cur.template next_var<double>();
+ else if (cur_type_desc == type_desc_from_type<long double>())
+ args_cur.template next_var<long double>();
+ else
+ return false;
+ return true;
+}
+#endif
+
} // namespace printf_core
} // namespace LIBC_NAMESPACE_DECL
diff --git a/libc/src/stdio/printf_core/printf_config.h b/libc/src/stdio/printf_core/printf_config.h
index 8a48abdd170ec..8f6ae8b41bc92 100644
--- a/libc/src/stdio/printf_core/printf_config.h
+++ b/libc/src/stdio/printf_core/printf_config.h
@@ -48,4 +48,11 @@
// LIBC_COPT_PRINTF_NO_NULLPTR_CHECKS
+#ifdef LIBC_COPT_PRINTF_MODULAR
+#define LIBC_PRINTF_MODULAR_DECL [[gnu::weak]]
+#else
+#define LIBC_PRINTF_MODULAR_DECL LIBC_INLINE
+#define LIBC_PRINTF_DEFINE_MODULAR
+#endif
+
#endif // LLVM_LIBC_SRC_STDIO_PRINTF_CORE_PRINTF_CONFIG_H
diff --git a/libc/src/stdio/printf_core/printf_main.h b/libc/src/stdio/printf_core/printf_main.h
index 57f29858d5298..c77922bb4f044 100644
--- a/libc/src/stdio/printf_core/printf_main.h
+++ b/libc/src/stdio/printf_core/printf_main.h
@@ -22,8 +22,8 @@ namespace LIBC_NAMESPACE_DECL {
namespace printf_core {
template <WriteMode write_mode>
-int printf_main(Writer<write_mode> *writer, const char *__restrict str,
- internal::ArgList &args) {
+int printf_main_modular(Writer<write_mode> *writer, const char *__restrict str,
+ internal::ArgList &args) {
Parser<internal::ArgList> parser(str, args);
int result = 0;
for (FormatSection cur_section = parser.get_next_section();
@@ -41,6 +41,15 @@ int printf_main(Writer<write_mode> *writer, const char *__restrict str,
return writer->get_chars_written();
}
+template <WriteMode write_mode>
+int printf_main(Writer<write_mode> *writer, const char *__restrict str,
+ internal::ArgList &args) {
+#ifdef LIBC_COPT_PRINTF_MODULAR
+ __asm__ __volatile__ (".reloc ., BFD_RELOC_NONE, __printf_float");
+#endif
+ return printf_main_modular(writer, str, args);
+}
+
} // namespace printf_core
} // namespace LIBC_NAMESPACE_DECL
diff --git a/libc/src/stdio/printf_core/vfprintf_internal.h b/libc/src/stdio/printf_core/vfprintf_internal.h
index 630de9d9d43dd..c9d6fce458409 100644
--- a/libc/src/stdio/printf_core/vfprintf_internal.h
+++ b/libc/src/stdio/printf_core/vfprintf_internal.h
@@ -67,7 +67,7 @@ LIBC_INLINE int file_write_hook(cpp::string_view new_str, void *fp) {
return WRITE_OK;
}
-LIBC_INLINE int vfprintf_internal(::FILE *__restrict stream,
+LIBC_INLINE int vfprintf_internal_modular(::FILE *__restrict stream,
const char *__restrict format,
internal::ArgList &args) {
constexpr size_t BUFF_SIZE = 1024;
@@ -76,7 +76,7 @@ LIBC_INLINE int vfprintf_internal(::FILE *__restrict stream,
buffer, BUFF_SIZE, &file_write_hook, reinterpret_cast<void *>(stream));
Writer writer(wb);
internal::flockfile(stream);
- int retval = printf_main(&writer, format, args);
+ int retval = printf_main_modular(&writer, format, args);
int flushval = wb.overflow_write("");
if (flushval != WRITE_OK)
retval = flushval;
@@ -84,6 +84,15 @@ LIBC_INLINE int vfprintf_internal(::FILE *__restrict stream,
return retval;
}
+LIBC_INLINE int vfprintf_internal(::FILE *__restrict stream,
+ const char *__restrict format,
+ internal::ArgList &args) {
+#ifdef LIBC_COPT_PRINTF_SPLIT
+ __asm__ __volatile__(".reloc ., BFD_RELOC_NONE, __printf_float");
+#endif
+ return vfprintf_internal_modular(stream, format, args);
+}
+
} // namespace printf_core
} // namespace LIBC_NAMESPACE_DECL
|
You can test this locally with the following command:git-clang-format --diff HEAD~1 HEAD --extensions cpp,h -- libc/src/stdio/generic/printf_modular.cpp libc/src/stdio/printf_core/float_impl.cpp libc/src/stdio/printf.h libc/src/stdio/printf_core/float_dec_converter.h libc/src/stdio/printf_core/float_dec_converter_limited.h libc/src/stdio/printf_core/float_hex_converter.h libc/src/stdio/printf_core/parser.h libc/src/stdio/printf_core/printf_config.h libc/src/stdio/printf_core/printf_main.h libc/src/stdio/printf_core/vfprintf_internal.hView the diff from clang-format here.diff --git a/libc/src/stdio/printf_core/float_dec_converter_limited.h b/libc/src/stdio/printf_core/float_dec_converter_limited.h
index 9804a3896..997131cc6 100644
--- a/libc/src/stdio/printf_core/float_dec_converter_limited.h
+++ b/libc/src/stdio/printf_core/float_dec_converter_limited.h
@@ -676,14 +676,15 @@ LIBC_INLINE int convert_float_dec_auto_typed(Writer<write_mode> *writer,
}
template <WriteMode write_mode>
-LIBC_PRINTF_MODULAR_DECL int convert_float_decimal(Writer<write_mode> *writer,
- const FormatSection &to_conv);
+LIBC_PRINTF_MODULAR_DECL int
+convert_float_decimal(Writer<write_mode> *writer, const FormatSection &to_conv);
template <WriteMode write_mode>
-LIBC_PRINTF_MODULAR_DECL int convert_float_dec_exp(Writer<write_mode> *writer,
- const FormatSection &to_conv);
+LIBC_PRINTF_MODULAR_DECL int
+convert_float_dec_exp(Writer<write_mode> *writer, const FormatSection &to_conv);
template <WriteMode write_mode>
-LIBC_PRINTF_MODULAR_DECL int convert_float_dec_auto(Writer<write_mode> *writer,
- const FormatSection &to_conv);
+LIBC_PRINTF_MODULAR_DECL int
+convert_float_dec_auto(Writer<write_mode> *writer,
+ const FormatSection &to_conv);
#ifdef LIBC_PRINTF_DEFINE_MODULAR
template <WriteMode write_mode>
diff --git a/libc/src/stdio/printf_core/float_hex_converter.h b/libc/src/stdio/printf_core/float_hex_converter.h
index fa7240668..5509d24d3 100644
--- a/libc/src/stdio/printf_core/float_hex_converter.h
+++ b/libc/src/stdio/printf_core/float_hex_converter.h
@@ -26,8 +26,8 @@ namespace LIBC_NAMESPACE_DECL {
namespace printf_core {
template <WriteMode write_mode>
-LIBC_PRINTF_MODULAR_DECL int convert_float_hex_exp(Writer<write_mode> *writer,
- const FormatSection &to_conv);
+LIBC_PRINTF_MODULAR_DECL int
+convert_float_hex_exp(Writer<write_mode> *writer, const FormatSection &to_conv);
#ifdef LIBC_PRINTF_DEFINE_MODULAR
template <WriteMode write_mode>
diff --git a/libc/src/stdio/printf_core/parser.h b/libc/src/stdio/printf_core/parser.h
index 5a1eea36b..1960f9a5c 100644
--- a/libc/src/stdio/printf_core/parser.h
+++ b/libc/src/stdio/printf_core/parser.h
@@ -296,8 +296,8 @@ public:
}
LIBC_PRINTF_MODULAR_DECL void write_float_arg_val(FormatSection §ion,
- LengthModifier lm,
- size_t conv_index);
+ LengthModifier lm,
+ size_t conv_index);
LIBC_PRINTF_MODULAR_DECL TypeDesc float_type_desc(LengthModifier lm);
LIBC_PRINTF_MODULAR_DECL bool advance_arg_if_float(TypeDesc cur_type_desc);
diff --git a/libc/src/stdio/printf_core/printf_main.h b/libc/src/stdio/printf_core/printf_main.h
index c77922bb4..bc9bd60dc 100644
--- a/libc/src/stdio/printf_core/printf_main.h
+++ b/libc/src/stdio/printf_core/printf_main.h
@@ -45,7 +45,7 @@ template <WriteMode write_mode>
int printf_main(Writer<write_mode> *writer, const char *__restrict str,
internal::ArgList &args) {
#ifdef LIBC_COPT_PRINTF_MODULAR
- __asm__ __volatile__ (".reloc ., BFD_RELOC_NONE, __printf_float");
+ __asm__ __volatile__(".reloc ., BFD_RELOC_NONE, __printf_float");
#endif
return printf_main_modular(writer, str, args);
}
diff --git a/libc/src/stdio/printf_core/vfprintf_internal.h b/libc/src/stdio/printf_core/vfprintf_internal.h
index c9d6fce45..e2808c04a 100644
--- a/libc/src/stdio/printf_core/vfprintf_internal.h
+++ b/libc/src/stdio/printf_core/vfprintf_internal.h
@@ -68,8 +68,8 @@ LIBC_INLINE int file_write_hook(cpp::string_view new_str, void *fp) {
}
LIBC_INLINE int vfprintf_internal_modular(::FILE *__restrict stream,
- const char *__restrict format,
- internal::ArgList &args) {
+ const char *__restrict format,
+ internal::ArgList &args) {
constexpr size_t BUFF_SIZE = 1024;
char buffer[BUFF_SIZE];
printf_core::WriteBuffer<Mode<WriteMode::FLUSH_TO_STREAM>::value> wb(
|
|
Sending a draft PR for this to get some early feedback. In particular, on the libc side, some things are missing:
Prev PR: #147431 |
mysterymath
force-pushed
the
users/mysterymath/modular-printf/libc
branch
from
42f5beb to
8d93269
Compare
mysterymath
changed the base branch from
main
to
users/mysterymath/modular-printf/clang
jhuber6
reviewed
Jul 7, 2025
mysterymath
force-pushed
the
users/mysterymath/modular-printf/clang
branch
from
2eedccf to
215f1f9
Compare
mysterymath
force-pushed
the
users/mysterymath/modular-printf/libc
branch
from
8d93269 to
04943cc
Compare
This was referenced Jul 8, 2025
If I understand what you mean by EDIT: I suppose that's not completely correct; I suppose you could include a weak definition that's just a byte or something in the module itself. It would get clobbered by the strongly-referenced real implementation aspect. But messy; is there a specific obstacle to |
Yes, ELF standard says that |
petrhosek
reviewed
Jul 8, 2025
| @@ -0,0 +1,41 @@ | |||
| #ifdef LIBC_COPT_PRINTF_MODULAR | |||
There was a problem hiding this comment.
This file needs the copyright header.
| @@ -682,6 +680,38 @@ template <typename ArgProvider> class Parser { | |||
| #endif // LIBC_COPT_PRINTF_DISABLE_INDEX_MODE | |||
| }; | |||
|
|
|||
| #ifdef LIBC_PRINTF_DEFINE_MODULAR | |||
| template <typename ArgParser> | |||
| void Parser<ArgParser>::write_float_arg_val(FormatSection §ion, | |||
There was a problem hiding this comment.
given that these are non-inline functions in a header, do they need to be in an anonymous namespace to avoid ODR issues? Here and elsewhere.
Comment on lines
+4
to
+7
| #define LIBC_PRINTF_DEFINE_MODULAR | ||
| #include "src/stdio/printf_core/float_dec_converter.h" | ||
| #include "src/stdio/printf_core/float_hex_converter.h" | ||
| #include "src/stdio/printf_core/parser.h" |
There was a problem hiding this comment.
given this, do you need to modify converter.h or converter_atlas.h?
| int printf_main(Writer<write_mode> *writer, const char *__restrict str, | ||
| internal::ArgList &args) { | ||
| #ifdef LIBC_COPT_PRINTF_MODULAR | ||
| __asm__ __volatile__ (".reloc ., BFD_RELOC_NONE, __printf_float"); |
There was a problem hiding this comment.
is there a way to do this with compiler attributes instead of asm?
| LIBC_INLINE int vfprintf_internal(::FILE *__restrict stream, | ||
| const char *__restrict format, | ||
| internal::ArgList &args) { | ||
| #ifdef LIBC_COPT_PRINTF_SPLIT |
There was a problem hiding this comment.
this seems like it wasn't updated
lntue
reviewed
Jul 8, 2025
| @@ -1122,11 +1122,23 @@ LIBC_INLINE int convert_float_dec_auto_typed(Writer<write_mode> *writer, | |||
| } | |||
| } | |||
|
|
|||
| template <WriteMode write_mode> | |||
| LIBC_PRINTF_MODULAR_DECL int | |||
| convert_float_decimal(Writer<write_mode> *writer, const FormatSection &to_conv); | |||
There was a problem hiding this comment.
It might be a separate PR, but can we change all the writer argument to reference instead of pointer?
mysterymath
force-pushed
the
users/mysterymath/modular-printf/libc
branch
from
04943cc to
028a89f
Compare
mysterymath
force-pushed
the
users/mysterymath/modular-printf/clang
branch
from
215f1f9 to
619dfb7
Compare
This adds LIBC_CONF_PRINTF_MODULAR, which causes floating point support (later, others) to be weakly linked into the implementation. __printf_modular becomes the main entry point of the implementaiton, an printf itself wraps __printf_modular. printf it also contains a BFD_RELOC_NONE relocation to bring in the float aspect. See issue #146159 for context.
mysterymath
force-pushed
the
users/mysterymath/modular-printf/clang
branch
from
619dfb7 to
e77a856
Compare
mysterymath
force-pushed
the
users/mysterymath/modular-printf/libc
branch
from
028a89f to
e6dd4bd
Compare
statham-arm
reviewed
Jul 9, 2025
| } // namespace LIBC_NAMESPACE_DECL | ||
|
|
||
| // Bring this file into the link if __printf_float is referenced. | ||
| extern "C" void __printf_float() {} |
There was a problem hiding this comment.
One risk with this strategy: what happens if this file is compiled with -ffunction-sections? Then __printf_float might be in a different code section from the template instantiations, and a linker might garbage-collect the functions you actually wanted, treating the weak references to them as not enough reason to keep them.
If I wanted to be sure of this technique, I'd either enforce via compile options that everything in this file is in the same code section, or else add further BFD_RELOC_NONE links from __printf_float to the payload functions, to make sure there's an unbroken chain of non-weak references from the import of __printf_float to the functions that actually do the work.
There was a problem hiding this comment.
One risk with this strategy: what happens if this file is compiled with
-ffunction-sections? Then__printf_floatmight be in a different code section from the template instantiations, and a linker might garbage-collect the functions you actually wanted, treating the weak references to them as not enough reason to keep them.
To my knowledge, this isn't possible. For LLD at least, the weakness of the reference isn't examined when determining whether or not a reference preserves a section. Weakness only controls whether or not an undefined or duplicate reference fails the link and whether an undefined reference extracts archive members. (There's some other differences with shared libraries unimportant to this case.)
I'd be surprised if other linkers differ; wasn't this usage the whole point of allowing undefined weak references?
There was a problem hiding this comment.
I'd be surprised if other linkers differ; wasn't this usage the whole point of allowing undefined weak references?
Certainly this usage in general is the whole point: a weak reference says "I won't insist that this function be included in the image, but if it has to be in the image anyway for some other reason, I'd like to call it."
The more specific question here is what counts as "has to be in the image anyway": do you need a non-weak reference to the section itself, or just to some section in the same object?
The linker in Arm's proprietary toolchain, armlink, takes the former attitude: garbage collection only follows non-weak references, so that if an object file containing many sections is pulled in to the link, the sections will then be independently GCed according to whether each one has a chain of non-weak references from the root. That's why I was concerned.
You may be right that armlink is an outlier and no linker this code cares about will behave the same way. But personally I'd check them all before being sure!
Comment on lines
+416
to
+418
| if (LIBC_CONF_PRINTF_MODULAR) | ||
| list(APPEND printf_srcs printf_modular.cpp) | ||
| endif() |
There was a problem hiding this comment.
would it make sense to have __printf_float as a separate entrypoint so that it can be controlled per-platform with the same mechanism as other entrypoints?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This adds LIBC_CONF_PRINTF_MODULAR, which causes floating point support (later, others) to be weakly linked into the implementation. __printf_modular becomes the main entry point of the implementaiton, an printf itself wraps __printf_modular. printf it also contains a BFD_RELOC_NONE relocation to bring in the float aspect.
See issue #146159 for context.